CA-403851 stop management server in Pool.eject () #6346
Conversation
lindig
force-pushed
the
private/christianlin/CA-403851
branch
from
9334508 to
4d60ae6
Compare
| @@ -2074,6 +2074,8 @@ let eject_self ~__context ~host = | |||
| ) | |||
| ) | |||
| (fun () -> Xapi_fuse.light_fuse_and_reboot_after_eject ()) ; | |||
| debug "%s: stop management server" __FUNCTION__ ; | |||
There was a problem hiding this comment.
Shouldn't this be done earlier? It's not clear when calling this server being to be problematic, but it looks like it should be between unplugging PBDs (line 1875) and declaring itself ot be the master (line 2016)
There was a problem hiding this comment.
I think anything before the API call returns is reasonable; a client should not make any assumptions before the call returns.
ocaml/xapi/xapi_mgmt_iface.mli
Outdated
| val stop : unit -> unit | ||
| (** Stop server for external API calls *) | ||
| end | ||
|
|
There was a problem hiding this comment.
I think it's strange to expose internal module Server with just one interface in the signature, while the real signature of Server contains more.
Suggest wrapping it as function stop_server and expose it in the upper module, which is Xapi_mgmt_iface
There was a problem hiding this comment.
I think it's fine to use modules to provide a clear name space.
There was a problem hiding this comment.
The status which stored in ref mode in xapi_mgmt_iface.ml L122 will not be updated. I don't know if it matters. I think it's better to use Server.(update __context Off) to implement this function.
There was a problem hiding this comment.
There is already a way to stop the server using the current interface, as used by the host.management_disable call: https://github.com/xapi-project/xen-api/blob/master/ocaml/xapi/xapi_host.ml#L1297.
There was a problem hiding this comment.
The update of status should be implemented by the stop function and seems to be missing.
There was a problem hiding this comment.
I'm now using the method @robhoes proposed and removed the commit that exposed Server.stop.
lindig
force-pushed
the
private/christianlin/CA-403851
branch
from
4d60ae6 to
68dc84d
Compare
|
BST have passed now |
|
BST after rebasing has now passed. |
We are stopping the management server at the end of the API call. This avoids clients connecting to this host before it has rebooted and become a pool master. Signed-off-by: Christian Lindig <christian.lindig@cloud.com>
lindig
force-pushed
the
private/christianlin/CA-403851
branch
from
68dc84d to
9b1b8d4
Compare
We are stopping the management server at the end of the API call. This avoids clients connecting to this host before it has rebooted and become a pool master.
Tested manually; running a BST now.